Conversation
|
|
👋 Fletch153, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
This comment was marked as outdated.
This comment was marked as outdated.
85618b5 to
fc81ec8
Compare
fc81ec8 to
715e340
Compare
…Event - Register JobInfo, RuntimeInfo, MetricsInfo, EndpointInfo, and ConfigurationItem entities
4dc81ad to
3a4f9be
Compare
platform/go.mod
Outdated
| @@ -0,0 +1,5 @@ | |||
| module github.com/smartcontractkit/chainlink-protos/platform | |||
There was a problem hiding this comment.
platform is a domain name for the protos emitted via Beholder.
| option go_package = "github.com/smartcontractkit/chainlink-protos/platform/bridge_status/v1"; | ||
|
|
||
| // BridgeStatusEvent represents the status data from an External Adapter | ||
| message BridgeStatusEvent { |
There was a problem hiding this comment.
This format will break the local-cre, and will require a decomposition similar to: #145 and needs to be supported by smartcontractkit/chainlink-testing-framework#2036 (cc @Tofel )
There was a problem hiding this comment.
I think Redpanda should support this as long as the root level proto is on the first place.
There was a problem hiding this comment.
I approved it assuming that we will only ever send BridgeStatusEvent messages. If that's the case it should work with the local CRE.
There was a problem hiding this comment.
Can confirm that the node is only sending BridgeStatusEvent messages (code here). This was added in smartcontractkit/chainlink#18634
|
|
||
| package bridge_status.v1; | ||
|
|
||
| option go_package = "github.com/smartcontractkit/chainlink-protos/platform/bridge_status/v1"; |
There was a problem hiding this comment.
if the go code belongs outside of the workflows domain, I'd suggest that this proto follow it. platform isn't the most meaningful domain name, if there's a domain more tightly scoped to the intent behind this change, let's use that.
There was a problem hiding this comment.
It's about getting more visibility into EAs, that's why we picked platform, since it's important for multiple products. Open to alternatives though!
See also DF-21668 for more context.
There was a problem hiding this comment.
data-feeds could be a more meaningful name than platform
What
Why
Notes